Propagate onboarding telemetry blob through broker success and failure paths, Fixes AB#3462876#3123
Conversation
|
✅ Work item link check complete. Description contains link AB#3462876 to an Azure Boards work item. |
There was a problem hiding this comment.
Pull request overview
This PR extends the onboarding telemetry feature so the broker-populated onboarding blob is preserved not only on success (AcquireTokenResult) but also when brokered flows fail/cancel by carrying the blob on BaseException and round-tripping it through the broker result bundle.
Changes:
- Add
BaseException.onboardingBlob(nullable) with accessor methods to carry the serialized onboarding telemetry blob on failure paths. - Update
MsalBrokerResultAdapterto serializeexception.onboardingBlobintoBrokerResultand restore it back onto the reconstructedBaseException. - Add unit tests validating blob round-trip behavior and null behavior parity; update
changelog.txtwith a MINOR entry.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| common4j/src/main/com/microsoft/identity/common/java/exception/BaseException.java | Adds onboardingBlob field + getter/setter to carry onboarding telemetry through exception-based failure paths. |
| common/src/main/java/com/microsoft/identity/common/internal/result/MsalBrokerResultAdapter.java | Serializes/deserializes onboarding blob for exceptions via BrokerResult to preserve it across IPC on failure/cancel outcomes. |
| common/src/test/java/com/microsoft/identity/common/internal/request/MsalBrokerResultAdapterTests.kt | Adds tests ensuring onboarding blob round-trips through exception→bundle→exception and stays null when unset. |
| changelog.txt | Records the change as a MINOR feature addition for vNext. |
wzhipan
changed the title
|
✅ Work item link check complete. Description contains link AB#3462876 to an Azure Boards work item. |
|
Overall change LGTM, ensure the build and tests are passing. Ensure on oneAuth side when reading blob check for Null or empty condition before operating on it. |
…metry-failure-extract # Conflicts: # changelog.txt
|
Rebased onto latest dev and resolved changelog conflicts. Please re-review when you get a chance. |
Prvnkmr337
left a comment
Prvnkmr337
left a comment
There was a problem hiding this comment.
Please check the coverage check failure, seems like false positive.
…test + corrupted Robolectric JAR cache + equal-coverage compare bug)
Makes
MsalBrokerResultAdaptercarry the onboarding telemetry blob symmetrically across every broker outcome (success, failure, cancel), closing the failure-path gap Veena flagged on #3111 and unblocking the broker recorder-lifecycle work.What this adds:
Failure path (was the original A3 scope)
BaseException.onboardingBlobfield with getter/setter (common4j).MsalBrokerResultAdapter.bundleFromBaseExceptionnow serializesexception.onboardingBlobinto the result bundle (symmetric with the existingClientDataInfoserialization right above it).MsalBrokerResultAdapter.getBaseExceptionFromBundleextracts the blob from the deserializedBrokerResultand attaches it to the reconstructed exception viasetOnboardingBlob.Success path (added in second commit)
MSAL-only overloads on
MsalBrokerResultAdapter:bundleFromAuthenticationResult(result, onboardingBlob, version)buildBrokerResultFromAuthenticationResult(result, onboardingBlob, version)The broker calls these from
AccountChooserActivity.returnSuccessToCallingActivity(upcoming PR) to ship the finalized recorder blob to the client. Existing overloads are preserved and delegate to the new ones withnull, so no behavior change for existing callers (includingAdalBrokerResultAdapterwhich doesn't need this).After this, OneAuth / Common can emit onboarding telemetry on the brokered success, failure, and cancel outcomes via:
acquireTokenResult.getOnboardingBlob()(success — already wired in Broker IPC plumbing for onboarding telemetry blob, Fixes AB#3568357 #3111)exception.getOnboardingBlob()(failure/cancel — new)Why now: Required by upcoming broker recorder-lifecycle work in PR #163 follow-ups, which writes the recorder blob on every termination path.
Tests:
MsalBrokerResultAdapterTestsadds 4 new round-trip tests (2 failure-path, 2 success-path). All 6testOnboardingBlob_*tests pass locally:./gradlew :common:testLocalDebugUnitTest --tests *MsalBrokerResultAdapterTests*testOnboardingBlob*Safety: Telemetry-only; never affects auth logic. No flight gate (blob is only present if a prior PR seeded it).
Fixes AB#3462876